Merged
Conversation
url 기반으로 판단하는게 확장성있는 판단일까
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRecruiting now defaults to true (absence of URL param => recruiting shown). The filter treats Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as "Filter UI\n(src/components/filtering)"
participant Page as "List Page\n(group / premium)"
participant URL as "URL SearchParams"
participant API as "Data Fetcher / API"
UI->>Page: changeFilter(recruiting: true|false)
Page->>URL: setSearchParam('recruiting' = 'true'|'false')
Note right of URL: absent param => pages default recruiting = true
URL-->>Page: updated params
Page->>API: fetchStudies(params with recruiting parsed (default true))
API-->>Page: studies
Page-->>UI: render updated list
sequenceDiagram
autonumber
participant Section as "MyParticipatingStudiesSection"
participant Auth as "Auth Hook"
participant MemberAPI as "useMemberStudyListQuery"
participant StudiesAPI as "useGetStudies"
participant GA as "Analytics"
Section->>Auth: ensureAuthenticated()
Auth-->>Section: memberId (or null)
Section->>MemberAPI: fetchMemberStudies(memberId, studyType)
MemberAPI-->>Section: memberStudyItems
Section->>StudiesAPI: fetchAllStudies(classification)
StudiesAPI-->>Section: allStudies
Section->>Section: derive participatingStudies (filter & cross-ref)
Section-->>UI: render StudyCard grid (up to 3) + "전체보기" if more
UI->>GA: handleStudyClick(studyId, hashedMemberId, title)
GA-->>UI: eventRecorded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai help |
ChatThere are 3 ways to chat with CodeRabbit:
CodeRabbit commands
Other keywords and placeholders
CodeRabbit configuration file (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/section/my-participating-studies-section.tsx`:
- Line 1: Run Prettier to fix formatting issues in this component file: execute
"prettier --write" (or your project's configured formatter) against
src/components/section/my-participating-studies-section.tsx (the
MyParticipatingStudiesSection component file) and commit the resulting changes
so the CI style check passes.
- Around line 24-26: The component calls hooks conditionally because of the
early return when memberId is falsy; move all hooks (useMemberStudyListQuery,
useGetStudies, useMemo, etc.) to the top of the component so they run
unconditionally, then handle the unauthenticated/render-empty case in the JSX
return instead of returning early, or alternatively keep hooks at top but add
the query option enabled: !!memberId (and pass memberId ?? 0 to the query input)
for useMemberStudyListQuery so queries are disabled until memberId exists.
🧹 Nitpick comments (1)
src/components/pages/premium-study-list-page.tsx (1)
81-128: Consider extracting shared URL filter logic into a custom hook.The
filterValues,updateSearchParams, andhandleFilterChangelogic is nearly identical betweenGroupStudyListPageandPremiumStudyListPage. A shareduseStudyFilterParams(basePath: string)hook could reduce duplication. This is optional and can be deferred.
| @@ -0,0 +1,196 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Run Prettier to fix formatting issues.
The CI pipeline failed due to code style issues. Run prettier --write on this file to resolve.
🧰 Tools
🪛 GitHub Actions: CI
[warning] 1-1: Code style issues found in the file. Run 'prettier --write' to fix.
[error] 1-1: Prettier formatting check failed. Command 'prettier --check .' exited with code 1. Run 'prettier --write' to fix code style issues in this file.
🤖 Prompt for AI Agents
In `@src/components/section/my-participating-studies-section.tsx` at line 1, Run
Prettier to fix formatting issues in this component file: execute "prettier
--write" (or your project's configured formatter) against
src/components/section/my-participating-studies-section.tsx (the
MyParticipatingStudiesSection component file) and commit the resulting changes
so the CI style check passes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/section/my-participating-studies-section.tsx`:
- Around line 81-89: The component assumes allStudiesData is an object with a
content array but useGetStudies returns the array directly; update the
participatingStudies logic to treat allStudiesData as an array (or change
useGetStudies to return the full response). Specifically, in the
participatingStudies computation (symbols: participatingStudies, allStudiesData,
participatingStudyIds, basicInfo.groupStudyId), change checks from
allStudiesData?.content to just allStudiesData and filter over allStudiesData
(e.g., allStudiesData.filter(...)) so the filter uses the actual array returned
by useGetStudies; alternatively modify useGetStudies to return { content: data }
so current code continues to work.
🧹 Nitpick comments (1)
src/components/section/my-participating-studies-section.tsx (1)
54-60: Avoid fetching studies for guests.Even though Line 95 returns
null,useGetStudieson Line 55 still fetches for unauthenticated users. Consider adding anenabledoption touseGetStudies(or gating the component higher) to skip this network call.Also applies to: 94-96
Comment on lines
+81
to
+89
| const participatingStudies = useMemo(() => { | ||
| if (!allStudiesData?.content || participatingStudyIds.size === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| // 내가 참여중인 스터디 ID에 해당하는 스터디만 필터링 | ||
| const filtered = allStudiesData.content.filter((study) => | ||
| participatingStudyIds.has(study.basicInfo?.groupStudyId ?? 0), | ||
| ); |
There was a problem hiding this comment.
Potential data-shape mismatch with useGetStudies.
useGetStudies currently returns data.content (array), but this component treats allStudiesData as { content: ... }. That makes allStudiesData?.content undefined, so participatingStudies becomes empty. Align the data shape or update the hook to return the full response.
✅ Suggested fix (treat data as array)
- const { data: allStudiesData, isLoading } = useGetStudies({
+ const { data: allStudiesData = [], isLoading } = useGetStudies({
classification,
page: 1,
pageSize: 100, // 충분히 많이 가져와서 필터링
recruiting: undefined, // 모든 상태 포함 (진행 중, 모집 중 모두)
});
// 내가 참여중인 스터디만 필터링 (최대 3개)
const participatingStudies = useMemo(() => {
- if (!allStudiesData?.content || participatingStudyIds.size === 0) {
+ if (participatingStudyIds.size === 0) {
return [];
}
// 내가 참여중인 스터디 ID에 해당하는 스터디만 필터링
- const filtered = allStudiesData.content.filter((study) =>
+ const filtered = allStudiesData.filter((study) =>
participatingStudyIds.has(study.basicInfo?.groupStudyId ?? 0),
);🤖 Prompt for AI Agents
In `@src/components/section/my-participating-studies-section.tsx` around lines 81
- 89, The component assumes allStudiesData is an object with a content array but
useGetStudies returns the array directly; update the participatingStudies logic
to treat allStudiesData as an array (or change useGetStudies to return the full
response). Specifically, in the participatingStudies computation (symbols:
participatingStudies, allStudiesData, participatingStudyIds,
basicInfo.groupStudyId), change checks from allStudiesData?.content to just
allStudiesData and filter over allStudiesData (e.g., allStudiesData.filter(...))
so the filter uses the actual array returned by useGetStudies; alternatively
modify useGetStudies to return { content: data } so current code continues to
work.
Contributor
Author
There was a problem hiding this comment.
우선 이 부분은 한 번 추후에 확인해볼게. 백엔드 적절한 API가 없어서 임시로 전체 스터디 다 부른 후 내 스터디 ID set 과 일치하는것을 필터링하고 있는데 추후에 새로운 API 를 만들게 되면 해결될거야. 이 기능이 나중에 빠질수도 있어서 my-participating-studies-section.tsx 는 임시 컴포넌트 정도로 생각하자. 추후에 스터디 개수가 늘어나면 리팩토링 해야돼.
There was a problem hiding this comment.
@seong-jin-jo, understood! I appreciate the context about this being a temporary implementation until the proper backend API is available.
Since this is technical debt that needs tracking, would you like me to create a GitHub issue to document the refactoring needed for my-participating-studies-section.tsx? The issue could outline:
- The current implementation (fetching all studies + client-side filtering)
- The proposed backend API improvement (endpoint accepting study IDs)
- Performance concerns when study count scales
This way it won't be forgotten when the backend team is ready to implement the proper API.
✏️ Learnings added
Learnt from: seong-jin-jo
Repo: code-zero-to-one/study-platform-client PR: 370
File: src/components/section/my-participating-studies-section.tsx:81-89
Timestamp: 2026-02-03T18:49:50.822Z
Learning: The component `src/components/section/my-participating-studies-section.tsx` is a temporary implementation that fetches all studies and filters them client-side against the user's participating study ID set. This approach is used due to the lack of a suitable backend API. The component should be refactored or potentially removed in the future when a proper backend API becomes available.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Contributor
Author
There was a problem hiding this comment.
응 어떻게할건지 몰라도 해봐
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create GitHub issue to track refactoring of my-participating-studies-section.tsx
gh issue create \
--repo code-zero-to-one/study-platform-client \
--title "Refactor my-participating-studies-section.tsx: Implement backend API for participating studies" \
--body "## Context
This component is currently a temporary implementation that needs refactoring once a proper backend API becomes available.
**Related PR:** `#370`
**Comment discussion:** https://github.com/code-zero-to-one/study-platform-client/pull/370#discussion_r2760428556
**File:** \`src/components/section/my-participating-studies-section.tsx\`
## Current Implementation
The component currently:
- Fetches **all studies** via \`useGetStudies\` (paginated, but fetching 100 items)
- Fetches user's participating study list via \`useMemberStudyListQuery\`
- Creates a Set of participating study IDs client-side
- Filters the complete study list against this ID set on the client
- Limits to 3 studies for display
## Problem
- **Performance concern:** As the number of studies grows, fetching and filtering all studies client-side becomes inefficient
- **Data type mismatch:** The participating studies list and full studies list have different data structures, requiring this workaround
- **Network overhead:** Unnecessary data transfer for studies the user isn't participating in
## Proposed Solution
Implement one of these backend API improvements:
### Option 1: Batch fetch by IDs
\`\`\`
GET /api/v1/group-studies/by-ids?ids=1,2,3&classification=GROUP_STUDY
\`\`\`
Returns full study details for the specified IDs only.
### Option 2: Enhance existing endpoint
Modify \`getMemberStudyList\` to return all information needed for \`StudyCard\` rendering, eliminating the need for a second API call.
## Next Steps
- [ ] Backend team implements appropriate API endpoint
- [ ] Refactor component to use new endpoint
- [ ] Remove or optimize client-side filtering
- [ ] Consider whether this feature should remain permanent or be removed
## Notes
- This feature may be removed in the future depending on product decisions
- Component should be treated as temporary until refactoring is completed
- Requested by `@seong-jin-jo`" \
--assignee seong-jin-jo
Length of output: 68
@seong-jin-jo, done! I've created issue #372 to track this refactoring work:
🔗 #372
The issue documents:
- Current implementation (fetching all studies + client-side filtering)
- Performance concerns as study count scales
- Two proposed backend API solutions
- Links back to this PR and discussion
The issue is assigned to you and includes checkboxes for tracking progress. Feel free to edit it if you'd like to adjust priorities or add more details!
🧠 Learnings used
Learnt from: seong-jin-jo
Repo: code-zero-to-one/study-platform-client PR: 370
File: src/components/section/my-participating-studies-section.tsx:81-89
Timestamp: 2026-02-03T18:49:50.822Z
Learning: The component `src/components/section/my-participating-studies-section.tsx` is a temporary implementation that fetches all studies and filters them client-side against the user's participating study ID set. This approach is used due to the lack of a suitable backend API. The component should be refactored or potentially removed in the future when a proper backend API becomes available.
This was referenced Feb 4, 2026
Open
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
url 기반으로 판단하는게 확장성있는 판단일까
🌱 연관된 이슈
☘️ 작업 내용
🍀 참고사항
스크린샷 (선택)
Summary by CodeRabbit
New Features
Bug Fixes
Chores